-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: git-hash injection on every build #1641
Conversation
* buhtz ***@***.***> [02-13-24 11:23]:
does this solve the problem?
patch -p1 /data/build/backintime/common/configure /home/paka/1641.patch
patching file /data/build/backintime/common/configure
Hunk #2 FAILED at 161.
Hunk #3 FAILED at 237.
2 out of 3 hunks FAILED -- saving rejects to file /data/build/backintime/common/configure.rej
patching file /data/build/backintime/common/configure
[SPLIT]
didn't work or I don't know how to apply?
|
Dear ptilopteri, please try to use correct upper case letters. Is is more gentle to the eyes of your readers.
I never used the email interface of Microsoft GitHub or the patch/diff files it provides. So I can not be of assistance here. Try it with that:
|
* buhtz ***@***.***> [02-13-24 15:07]:
Dear ptilopteri,
please try to use correct upper case letters. Is is more gentle to the eyes of your readers.
> didn't work or I don't know how to apply?
I never used the email interface of Microsoft GitHub or the patch/diff files it provides. So I can not be of assistance here.
I do work directly with "git". Just clone my forked repository, checkout the bugfix branch and got for it.
Try it with that:
```
$ git clone https://github.com/buhtz/backintime.git
$ cd backintime
$ git checkout fix/fix1637
$ cd common && ./configure && make && sudo make install
$ cd ../qt && ./configure && make && sudo make install
$ backintime --version
```
Message ID: ***@***.***>
does that still need to be patched with ~/1592_workaround.txt ?
backintime 1.4.4-dev.6d294fb0
that is very nice. Good to go.
tks
|
I am not aware of a file named like that. If you refer to Issue #1592 (assigned to aryoda, not me) I would answer no, because that Issue is not closed (solved) yet. |
@ptilopteri The patch is only a workaround and must still be applied for each BiT version built from the |
@buhtz The current code of this PR does call the I think both targets have pros and cons and we simply have to decide (or toss a coin ;-)
I am "voting" for the |
common/configure
Outdated
@@ -161,7 +161,8 @@ printf "DEST=\$(DESTDIR)\$(PREFIX)\n\n" >> ${MAKEFILE} | |||
|
|||
printf "all:\tbuild\n\n" >> ${MAKEFILE} | |||
|
|||
printf "build:\ttranslate compress\n\n" >> ${MAKEFILE} | |||
printf "build:\ttranslate compress\n" >> ${MAKEFILE} | |||
printf "\t(cd .. && ./updateversion.sh)\n\n" >> ${MAKEFILE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think changing the directory within the script may have side effects on all other relative paths in commands called subsequently (= using the wrong path) - if any are called
AFAIR a solution for this would be to cd
in the called updatevesion.sh
script (into the folder of the script).
I need to search for a bash
code for this (can't find it ATM).
Or maybe: Make sure this line is always called at the end (and a comment should explain this and warn not to move it into the middle of the generated Makefile
code). Currently it looks like it is the last command of the target...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is IMHO no problem because this "cd" is temporary because of the (...)
. It is activte only inside the brackets.
$ ls && (cd .. && ls) && ls
huhu.txt
downdir/ infoobar.txt
huhu.txt
It seems to be POSIX and works on several shells. The make system do use "sh" by default. There is also the pushd
command but it works only on bash
. You would do ls && pushd .. && ls && pushd +1 && ls
on bash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic - looks very good (didn't know the parentesis semantics until now :-)
Thanks for reviewing. I moved it from "build" to "install" target. But I kept it as the first action in that target because it need to be done before the py-files are copied via The "cd" is IMHO no problem because is temporary in that script. |
Dear @ptilopteri,
does this solve the problem?
Close #1640
Close #1637